Sediakan pengelolaan QR Code pengganti TTE#1580
Conversation
- Generate QR Code per surat menggunakan endroid/qr-code - Sematkan QR Code di halaman terakhir surat menggunakan FPDI - QR Code berisi tautan ke halaman verifikasi surat - Simpan hash SHA-256 dari isi file untuk verifikasi keaslian - Sediakan fitur verifikasi file unggahan (unggah PDF, sistem cocokkan hash) - Penyematan QR Code hanya bisa dilakukan oleh pengguna camat - Tampilkan kolom hash di arsip surat dan halaman verifikasi Closes OpenSID#219
There was a problem hiding this comment.
mas @lukman48 fix conflict dulu, untuk branch tujuan gunakan rilis-dev bukan master
Code Review: fitur/qr-code-tte vs rilis-devBranch: SummaryMenambahkan fitur penandatanganan surat menggunakan QR Code sebagai alternatif TTE (Tanda Tangan Elektronik) BSrE, termasuk penghitungan hash SHA-256 untuk verifikasi keaslian file, halaman verifikasi surat, dan integrasi QR Code pada PDF surat. Verdict: [x] Request Changes | [ ] Approve | [ ] Comment Critical Issues (Must Fix)1.
|
pandigresik
left a comment
pandigresik
left a comment
There was a problem hiding this comment.
lihat komentar sebelumnya
- Fix race condition: rename file signed sebelum hapus file asal - Tambahkan LogTte audit logging untuk penandatangan QR Code - Fix temp file naming collision dengan uniqid() - Hapus @Unlink error suppression, gunakan file_exists check - Escape HTML di DataTable hash column dengan e() - Gunakan enum LogVerifikasiSurat::ProsesTTE bukan magic number 4 - Null safety untuk $profil dan $surat->penduduk - Pindahkan route verifikasi ke publik (tanpa login)
Revisi Code ReviewTerima kasih atas reviewnya yang sangat detail @pandigresik. Berikut respon terhadap semua issue yang telah diperbaiki: Critical Issues1. Race condition pada operasi file ( Sudah diperbaiki. Urutan operasi file diubah:
Major Issues1. Tidak ada pencatatan audit LogTte ( Sudah ditambahkan. Sekarang 2. Temp file naming collision ( Sudah diperbaiki. Nama file temporary menggunakan $qrTempPath = public_path('storage/surat/qr_temp_' . uniqid() . '_' . $surat->id . '.png');3. Error suppression pada Sudah diperbaiki. 4. Raw HTML tanpa escaping di DataTable ( Sudah ditambahkan return '<code style="font-size: 10px;">' . e(substr($row->file_hash, 0, 16)) . '...</code>';Minor Issues1. Magic number Sudah diganti dengan enum 2. Sudah diganti dengan 3. Sudah ditambahkan fallback Jawaban untuk PertanyaanQ1: Verifikasi publik vs internal — Halaman verifikasi berada di dalam middleware Tidak disengaja. Sudah diperbaiki dengan memindahkan route verifikasi ke luar middleware Q2: Pencatatan LogTte — Apakah ada alasan mengapa penandatanganan QR Code tidak dicatat ke Tidak ada alasan khusus, ini memang terlewat. Sekarang sudah ditambahkan pencatatan Q3: Race condition file — Apakah ada mekanisme di aplikasi yang mencegah dua user menandatangani surat yang sama secara bersamaan? Tidak ada mekanisme eksplisit. Namun dengan perbaikan Files Changed
|
🔐 Security Audit — PR #1580 (QR Code pengganti TTE)
✅ Kesimpulan: APPROVE WITH MINOR CHANGESSecara arsitektur implementasi ini aman dan mengikuti pola otorisasi yang sudah ada. Tidak ditemukan webshell/malware/injection. Namun ada 1 isu HIGH dan 2 MEDIUM yang sebaiknya diperbaiki sebelum merge. 📊 Ringkasan Temuan
🟠 HIGH-1 — Kebocoran PII pada Verifikasi PublikRoute
Siapa pun yang punya file PDF asli bisa upload & dapatkan data pribadi warga tanpa autentikasi → melanggar prinsip perlindungan data (UU PDP). Fix: Halaman verifikasi publik hanya boleh menampilkan status 🟡 MEDIUM-1 — Path Traversal$file_path = public_path("storage/surat/{$surat->file}");
$pdf->setSourceFile($file_path);
rename($signed_path, $file_path);Nilai Fix: $file_path = public_path('storage/surat/' . basename($surat->file));🟡 MEDIUM-2 — Throttle Endpoint PublikRoute::post('/surat/verifikasi', [...])->middleware('throttle:10,1');🔵 LOW-1 — Jangan kirim pesan exception ke client// ganti
'pesan_error' => $e->getMessage(),
// menjadi
'pesan_error' => 'Terjadi kesalahan saat menandatangani surat.',✅ Aspek yang Sudah Baik
🔗 Relasi dengan Issue #219 (dan #217)
Saran: Jangan auto-close #219 dengan merge ini. Tandai sebagai solusi sementara (interim) & buka issue lanjutan untuk TTE BSRE sesungguhnya. Tambahkan disclaimer di view bahwa ini QR verifikasi internal, bukan TTE bersertifikat BSRE. Verdict: ✅ Layak merge setelah perbaiki HIGH-1 + MEDIUM-1 (< 30 menit). |
| { | ||
| public function up() | ||
| { | ||
| Schema::table('das_log_surat', function (Blueprint $table) { |
There was a problem hiding this comment.
tabel ini belum memiliki reference ke surat tertentu, mungkin bisa ditambahkan column surat_id atau semisalnya
| @@ -0,0 +1,70 @@ | |||
| @extends('layouts.dashboard_template') | |||
There was a problem hiding this comment.
halaman public tapi menggunakan layout admin
| @@ -0,0 +1,43 @@ | |||
| @extends('layouts.dashboard_template') | |||
There was a problem hiding this comment.
halaman public tapi menggunakan layout admin
| return view('surat.verifikasi.index', compact('page_title', 'page_description')); | ||
| } | ||
|
|
||
| public function verifikasiStore(Request $request) |
There was a problem hiding this comment.
gunakan formRequest seperti pada method lain untuk validasi form
Code Review:
|
| Metode | Return Type yang Diexpected |
|---|---|
index() |
: View |
show($id) |
: View |
ditolak() |
: View |
download($id) |
: BinaryFileResponse |
setujui($id) |
: JsonResponse |
tolak(Request $request, $id) |
: JsonResponse |
passphrase(Request $request, $id) |
: JsonResponse |
tandatanganQr($id) |
: JsonResponse |
getData() |
: JsonResponse |
getDataDitolak() |
: JsonResponse |
9. Minor — Tidak ada type hint pada parameter $id
File: app/Http/Controllers/Surat/PermohonanController.php, app/Http/Controllers/Surat/SuratController.php
Route sudah menggunakan nama model sebagai parameter ({surat}), yang memungkinkan Laravel route model binding. Namun semua controller method mendefinisikan $id tanpa type hint:
// Routes (web.php:866-873):
Route::get('show/{surat}', ...);
Route::get('download/{surat}', ...);
Route::get('setujui/{surat}', ...);
Route::post('tolak/{surat}', ...);
Route::post('passphrase/{surat}', ...);
Route::post('tandatangan-qr/{surat}', ...);
// Controller — tanpa type hint:
public function show($id) // PermohonanController:136
public function download($id) // PermohonanController:162, SuratController:92
public function setujui($id) // PermohonanController:179
public function tolak(Request $request, $id) // PermohonanController:209
public function passphrase(Request $request, $id) // PermohonanController:228
public function tandatanganQr($id) // PermohonanController:298
public function qrcode($id) // SuratController:139Jika menggunakan route model binding (Surat $surat), Laravel akan:
- Auto-resolve model dari route parameter
- Auto-return 404
ModelNotFoundExceptionjika tidak ditemukan - Menghilangkan kebutuhan
Surat::findOrFail($id)secara manual
Bandingkan dengan pola yang sudah ada di ArtikelController:
public function edit(Artikel $artikel): View
public function update(ArtikelRequest $request, Artikel $artikel): RedirectResponse
public function destroy(Artikel $artikel): RedirectResponse10. Minor — Type hint $notif pada response() helper
File: app/Http/Controllers/Surat/PermohonanController.php:395
protected function response($notif = [])Parameter $notif tidak memiliki type hint. Seharusnya array $notif atau lebih spesifik lagi menggunakan type hint array keys jika memungkinkan.
11. Minor — Gaya akses parameter request tidak konsisten
Controller mencampur dua gaya:
$request['keterangan'](array access) — line 215, 253$request->file('file')(method call) — diSuratController
Seharusnya konsisten menggunakan $request->input() atau $request->validated() (dengan FormRequest).
Prioritas Perbaikan
| # | Severity | Issue | Effort |
|---|---|---|---|
| 1 | High | File overwrite sebelum DB commit | Sedang |
| 2 | Medium | Exception message bocor ke client | Kecil |
| 3 | Medium | Layout admin pada halaman publik | Sedang |
| 4 | Medium | File sementara tidak dibersihkan | Kecil |
| 5 | Medium | Validasi keterangan di tolak() |
Kecil |
| 6 | Medium | Validasi passphrase di passphrase() |
Kecil |
| 7 | Minor | LogTte tanpa surat_id |
Kecil |
| 8 | Minor | Return type declaration | Kecil |
| 9 | Minor | Tidak ada type hint pada parameter $id |
Kecil |
| 10 | Minor | Type hint $notif pada response() |
Kecil |
| 11 | Minor | Gaya akses request tidak konsisten | Kecil |
- High: pindahkan rename() setelah DB::commit() (data integrity) - Medium: exception message generik, hapus @Unlink, cleanup temp files - Medium: validasi keterangan (tolak) dan passphrase - Medium: rate-limit throttle:10,1 di endpoint publik - Medium: path traversal - basename() pada ->file - Medium: layout publik untuk halaman verifikasi - High: PII tidak ditampilkan di halaman verifikasi publik - Minor: route model binding, return types, type hints - Minor: LogTte surat_id (fillable + migration) - Minor: konsistensi akses request (->input)
| </li> | ||
| <li {{ Request::is(['surat/arsip*']) ? 'class=active' : '' }}><a href="{{ route('surat.arsip') }}"><i class="fa fa-folder-open"></i>Arsip</a> | ||
| </li> | ||
| <li {{ Request::is(['surat/verifikasi*']) ? 'class=active' : '' }}><a href="{{ route('surat.verifikasi') }}"><i class="fa fa-qrcode"></i>Verifikasi</a> |
There was a problem hiding this comment.
apakah ini masih diperlukan ? Karena saya lihat ini route publik bukan route untuk admin
|
Mohon lakukan test ulang setelah perbaikan 
|
| } | ||
|
|
||
| public function tolak(Request $request, $id) | ||
| public function tolak(Request $request, Surat $surat): JsonResponse |
There was a problem hiding this comment.
masih belum menggunakan formRequest untuk validasi
| } | ||
|
|
||
| public function passphrase(Request $request, $id) | ||
| public function passphrase(Request $request, Surat $surat): JsonResponse |
There was a problem hiding this comment.
gunakan formRequest untuk validasi
| } | ||
| } | ||
|
|
||
| public function tandatanganQr(Surat $surat): JsonResponse |
There was a problem hiding this comment.
tambahkan testing untuk method baru, lengkapi pada tests/Feature/PermohonanControllerTest.php
| @@ -37,21 +37,26 @@ | |||
| use App\Models\Profil; | |||
There was a problem hiding this comment.
buatkan unit/feature test untuk controller baru
| return view('surat.qrcode', compact('surat', 'profil')); | ||
| } | ||
|
|
||
| public function verifikasi(): View |
There was a problem hiding this comment.
buatkan unit/feature test untuk method baru
|
Tambahkan juga video yang menunjukkan bahwa fitur surat dengan QR Code ini berhasil dijalankan pada aplikasi |
4 participants
Deskripsi
Menyediakan fitur pengelolaan QR Code sebagai pengganti TTE (Tanda Tangan Elektronik) dari BSRE. Fitur ini memungkinkan camat untuk menandatangani surat dengan QR Code yang dihasilkan sendiri oleh OpenDK tanpa bergantung pada layanan eksternal BSRE.
Masalah Terkait
Close #219
Perubahan yang dilakukan
Library Baru
Migration
file_hash(SHA-256) ke tabeldas_log_suratModel
file_hashke$fillableController
PermohonanController: Menambahkan method
tandatanganQr()yang:SuratController: Menambahkan method:
verifikasi(): Menampilkan halaman unggah file untuk verifikasiverifikasiStore(): Memproses unggahan, mencocokkan hash dengan databaseRoutes
POST surat/permohonan/tandatangan-qr/{surat}: Endpoint tanda tangan QR CodeGET surat/verifikasi: Halaman verifikasi suratPOST surat/verifikasi: Proses verifikasi unggahan fileViews
Alur Kerja
Daftar Periksa